Skip to content

Conversation

@stof
Copy link
Member

@stof stof commented Oct 28, 2025

This migrates the testsuite to use PHPUnit 11.

I'm now installing phpunit through composer rather than using simple-phpunit as it is much simpler: matthiasnoback/symfony-config-test already brings us a transitive dependency anyway (for which we needed to add a conflict rule to prevent issues).

I'm still registering Symfony\Bridge\PhpUnit\SymfonyExtension from the symfony phpunit bridge in our config. The feature I wanted is the registration of the DebugClassLoader of symfony/error-handler to report deprecations detecting by the DebugClassLoader (which was previously done for root tests where error-handler is installed by FrameworkBundle but not for component tests where it was not installed). It might also be useful if we want to use the ClockMock or DnsMock features in the future.

@stof stof force-pushed the phpunit_11 branch 5 times, most recently from 78ca057 to 5c2ff5c Compare October 28, 2025 23:05
@stof
Copy link
Member Author

stof commented Oct 28, 2025

The remaining failures for root tests are because our changelog tests currently don't allow changing the composer.json without changelog entries. But here, I'm only changing dev requirements.
@jderusse should we add the composer.json to the exclusion list ?

Another issue affect only the root tests with dev stability is a bug in the symfony/phpunit-bridge dev version. I have a fix pending at symfony/symfony#62196

"phpunit/phpunit": "^11.5.42",
"symfony/cache": "^4.4 || ^5.0 || ^6.0 || ^7.0 || ^8.0",
"symfony/error-handler": "^7.3.2 || ^8.0",
"symfony/framework-bundle": "^5.4.45 || ^6.4.13 || ^7.1.6",
Copy link
Member Author

@stof stof Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This requirement on FrameworkBundle is meant to fix our --prefer-lowest CI job by restrict the lowest bound (for the dependency coming from nyholm/symfony-bundle-test) so that we have a version including the change in symfony/framework-bundle@1c11e27

this change combined with the fact that I install symfony/runtime solves the issue that booting the bundle would attempt at replacing the error handler, causing issues with PHPUnit 11 (this is why I don't let the composer plugin run for the runtime component: I won't care about it).

failOnNotice="true"
>
<coverage>
<source ignoreSuppressionOfDeprecations="true" ignoreIndirectDeprecations="true">
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this package gets indirect deprecations (from illuminate/container) when using the DebugClassLoader. I'm ignoring them here as I still want the DebugClassLoader for our own code.

@jderusse
Copy link
Member

jderusse commented Oct 29, 2025

The remaining failures for root tests are because our changelog tests currently don't allow changing the composer.json without changelog entries. But here, I'm only changing dev requirements. @jderusse should we add the composer.json to the exclusion list ?

I'm mixed feelling... the changelog is used by the release script to guess the next version.

So components with only change in composer will not be released. This is fine now because the changelog has a pending entry for php 8.2.
But I wonder if we could have case where we bump a dependency (and only update composer.json) that would require a new release 🤔 (ie. add a conflict for an incompatible version of a dependency, or replace an unmaintained dep by its forks, ...)
Those are really edge cases, but the test is here to avoid forgetting this.

I'm fine with merging this PR with red test, but I think we should not change the test.

@stof
Copy link
Member Author

stof commented Oct 29, 2025

I'm fine with merging this PR with red test, but I think we should not change the test.

This would require disabling the branch protection rule though (unless you have permission to bypass it, which I don't).
But this is indeed an option.

@jderusse
Copy link
Member

I'm fine with merging this PR with red test, but I think we should not change the test.

This would require disabling the branch protection rule though (unless you have permission to bypass it, which I don't). But this is indeed an option.

I have permission to do it.
Shall we wait for symfony/symfony#62196 to be merged

@stof
Copy link
Member Author

stof commented Oct 29, 2025

Shall we wait for symfony/symfony#62196 to be merged

it would make sense, yes. Otherwise the job will fail for all PRs. But I expect it to be merged soon.

@stof
Copy link
Member Author

stof commented Oct 29, 2025

@jderusse the Symfony PR is now merged, so we are good to go.

@jderusse jderusse merged commit e1d6459 into async-aws:master Oct 30, 2025
15 of 19 checks passed
@jderusse
Copy link
Member

thank you @stof

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants